feat(nextjs): remove tracing from pages router API routes #18394

logaretm · 2025-12-03T10:13:27Z

This is a big PR because of the tests added, the logic changes themselves are pretty minimal, so make sure to filter out the files when reviewing.

Actual Changes

Removed traces from wrapApiHandlerWithSentry for both the server runtime and the edge runtime

Tests Added

Since we are dropping explicit trace wrapping logic from the pages router templates, we needed to have tests on Next 16 to make sure the page router there still works in both Turbopack and Webpack with minimal differences in the quality of the traces.

We have around 36 test case to check, but only 20 can pass at this time due to the wrapping logic still present, ideally as we move towards removing templates entirely, we can then unskip those tests and tighten many of the assertions we have to accommodate for it.

I added TODOs and a TEST_STATUS.md document to detail what we should update once the SDK drops more of the wrapping logic it currently has.

Closes #18450 (added automatically)

linear · 2025-12-03T10:13:31Z

JS-1209 Remove tracing from Pages Router API handler template

github-actions · 2025-12-03T12:07:22Z

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,022	-	8,535	+29%
GET With Sentry	1,954	18%	1,608	+22%
GET With Sentry (error only)	7,856	71%	5,850	+34%
POST Baseline	1,145	-	1,166	-2%
POST With Sentry	578	50%	546	+6%
POST With Sentry (error only)	1,022	89%	1,023	-0%
MYSQL Baseline	4,044	-	3,188	+27%
MYSQL With Sentry	530	13%	427	+24%
MYSQL With Sentry (error only)	3,313	82%	2,607	+27%

View base workflow run

packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts

Copilot

Pull request overview

This PR removes explicit tracing instrumentation from Pages Router API route wrappers (wrapApiHandlerWithSentry) for both Node.js server and Edge runtimes, relying instead on Next.js's built-in OpenTelemetry instrumentation to create transaction spans. The wrappers now only handle error capture, transaction name setting on isolation scope, and route backfilling.

Key Changes:

Removed manual span creation (startSpanManual, startSpan) from API route wrappers
Wrappers now set transaction names on isolation scope and use route backfill attributes for parameterized route names
Updated test expectations to reflect origin change from 'auto.http.nextjs' to 'auto' (from Next.js OTEL)
Added comprehensive Next.js 16 Pages Router test application with 36+ test cases

Reviewed changes

Copilot reviewed 74 out of 76 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts`	Removed tracing logic; now only captures errors and sets transaction metadata via isolation scope and route backfill
`packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts`	Removed tracing logic from edge runtime wrapper; simplified to error monitoring only
`packages/nextjs/src/server/index.ts`	Added `SEMANTIC_ATTRIBUTE_SENTRY_SOURCE` to route backfill logic to ensure proper transaction source attribution
`packages/nextjs/test/config/withSentry.test.ts`	Deleted unit test that verified explicit span creation (no longer applicable)
`dev-packages/e2e-tests/test-applications/nextjs-13/tests/*/.test.ts`	Updated assertions to expect `'auto'` origin instead of `'auto.http.nextjs'`
`dev-packages/e2e-tests/test-applications/create-next-app/tests/*/.test.ts`	Updated assertions for new tracing behavior with flexible parent span matching
`dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/edge-route.test.ts`	Removed runtime context assertions and skipped scope isolation test
`dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/*/`	New comprehensive test application with 36 test cases for Next.js 16 Pages Router covering both webpack and turbopack
`dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/TEST_STATUS.md`	Detailed documentation of test status, known issues, and future work items

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.../e2e-tests/test-applications/nextjs-16-pages-dir/tests/server/excluded-api-endpoints.test.ts

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/tests/edge-route.test.ts

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/tests/pages-ssr-errors.test.ts

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/tests/proxy.test.ts

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/tests/transactions.test.ts

...kages/e2e-tests/test-applications/nextjs-16-pages-dir/pages/pages-router/ssr-error-class.tsx

...packages/e2e-tests/test-applications/nextjs-16-pages-dir/pages/pages-router/ssr-error-fc.tsx

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/proxy.ts

logaretm · 2025-12-10T09:25:34Z

packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts

+        // as the runtime freezes as soon as the error is thrown below
+        await flushSafelyWithTimeout();
+
+        // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
+        // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
+        // the error as already having been captured.)
+        throw objectifiedErr;
+      }
    },
  });
 }


I switched to a blocking await here before throwing which should work better than calling waitUntil then throwing as the runtime can shutdown as soon as an unhandled error like these are thrown.

This matches the logic we have in the other API handler wrapper.

packages/nextjs/src/server/index.ts

cursor · 2025-12-16T12:28:34Z

packages/nextjs/src/edge/index.ts

+      // Set transaction name on isolation scope to ensure parameterized routes are used
+      // The HTTP server integration sets it on isolation scope, so we need to match that
+      const method = event.request?.method || 'GET';
+      path = path ?? event.request?.url ?? '/';


Bug: Nullish coalescing won't handle empty path string

The path variable is assigned from String(...).replace(...).trim() on line 161, which always returns a string (possibly empty ""). On line 165, path = path ?? event.request?.url ?? '/' uses nullish coalescing (??), which only falls back for null/undefined, not for empty strings. If the span name is exactly 'executing api route (pages)' with no route suffix, path becomes "" and the fallback to event.request?.url or / never triggers. This results in malformed transaction names like "GET " instead of "GET /". Using the logical OR operator (||) would correctly handle empty strings.

logaretm force-pushed the awad/js-1209-remove-tracing-from-pages-router-api-2 branch 2 times, most recently from 2af20e5 to 2516846 Compare December 3, 2025 10:49

logaretm force-pushed the awad/js-1209-remove-tracing-from-pages-router-api-2 branch 3 times, most recently from c055f2d to 8102dd9 Compare December 9, 2025 14:43

This was linked to issues Dec 9, 2025

Remove tracing from Pages Router API handler template #18309

Open

Remove tracing from Edge API handler #18310

Open

logaretm marked this pull request as ready for review December 10, 2025 08:45

Copilot AI review requested due to automatic review settings December 10, 2025 08:45

github-actions bot mentioned this pull request Dec 10, 2025

feat(nextjs): remove tracing from pages router API routes #18450

Closed

logaretm requested a review from chargome December 10, 2025 08:45

Copilot started reviewing on behalf of logaretm December 10, 2025 08:45 View session

sentry bot reviewed Dec 10, 2025

View reviewed changes

packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts Show resolved Hide resolved

Copilot AI reviewed Dec 10, 2025

View reviewed changes

cursor bot reviewed Dec 10, 2025

View reviewed changes

dev-packages/e2e-tests/test-applications/nextjs-16-pages-dir/proxy.ts Outdated Show resolved Hide resolved

sentry bot reviewed Dec 10, 2025

View reviewed changes

cursor bot reviewed Dec 10, 2025

View reviewed changes

packages/nextjs/src/server/index.ts Show resolved Hide resolved

logaretm added 8 commits December 16, 2025 12:08

feat(nextjs): remove tracing from pages router API routes

e2fef71

fix: metadata processing

6fce173

fix: set the transaction name on the isolation scope level

ec986a8

tests: try without paramaterization

31fac7a

fix: parameterization backfill

eb4c84d

test: update expectations and fix lint issues

596150f

test: update expectations

ea4d841

feat: drop tracing from edge pages runtime wrapping

bdb4be3

logaretm force-pushed the awad/js-1209-remove-tracing-from-pages-router-api-2 branch from 949da51 to bdb4be3 Compare December 16, 2025 10:08

logaretm added 2 commits December 16, 2025 13:31

test: fix assertion expectation to expect parent id span

ceaa144

tests: adjust assertions to be consistent

03e7627

feat: remove transaction and SDK metadata setting from the wrapper

9c04b47

logaretm force-pushed the awad/js-1209-remove-tracing-from-pages-router-api-2 branch from 6de8d44 to 9c04b47 Compare December 16, 2025 12:24

cursor bot reviewed Dec 16, 2025

View reviewed changes

logaretm marked this pull request as draft December 16, 2025 14:34

test: update next 13 assertions

ecfe19b

logaretm force-pushed the awad/js-1209-remove-tracing-from-pages-router-api-2 branch from 4538dfe to ecfe19b Compare December 16, 2025 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(nextjs): remove tracing from pages router API routes #18394

feat(nextjs): remove tracing from pages router API routes #18394

logaretm commented Dec 3, 2025 •

edited by github-actions bot

Loading

Uh oh!

linear bot commented Dec 3, 2025

Uh oh!

github-actions bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

logaretm Dec 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

cursor bot Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

feat(nextjs): remove tracing from pages router API routes #18394

Are you sure you want to change the base?

feat(nextjs): remove tracing from pages router API routes #18394

Conversation

logaretm commented Dec 3, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Actual Changes

Tests Added

Uh oh!

linear bot commented Dec 3, 2025

Uh oh!

github-actions bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

node-overhead report 🧳

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

logaretm Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot Dec 16, 2025

Choose a reason for hiding this comment

Bug: Nullish coalescing won't handle empty path string

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

logaretm commented Dec 3, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Dec 3, 2025 •

edited

Loading

logaretm Dec 10, 2025 •

edited

Loading